Skip to content

Add support for reading ORAS5 data#2422

Merged
bouweandela merged 51 commits into
mainfrom
oras5
Jun 22, 2026
Merged

Add support for reading ORAS5 data#2422
bouweandela merged 51 commits into
mainfrom
oras5

Conversation

@jmalles

@jmalles jmalles commented May 17, 2024

Copy link
Copy Markdown
Contributor

Description

Fix for ORAS5. It includes the option to make the 'irregular' (2d) grid 'unstructured' (1d) and UGRID-compliant. To do so, one needs to add the extra facet 'make_unstructured: true' and/or 'ugrid: true' in the recipe.

The mesh description ('horizontal_grid' in the recipe) needs to be read from a file (oras5_mesh.zip), which is based on a file that can be downloaded here. Since ORAS5 data is on the ORCA025 (Arakawa-C grid), there are different mesh files for tracers (T), and zonal/meridional velocities (u/v).

An example recipe for ORAS5 can be found here.

Some already downloaded data are stored here:
/work/bd1083/b382555/extraobsraw/Tier2/ORAS5/data

(for 3D data in all_levels so far usually only short time periods are downloaded, due to their size)

The grid files are also stored on Levante:
/work/bd1083/b382555/extraobsraw/Tier2/ORAS5/grids

Link to documentation: https://esmvaltool--2422.org.readthedocs.build/projects/ESMValCore/en/2422/quickstart/find_data.html#oras5


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.


To help with the number pull requests:

@codecov

codecov Bot commented Dec 2, 2024

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.23%. Comparing base (23e69e9) to head (f927f25).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2422      +/-   ##
==========================================
+ Coverage   96.19%   96.23%   +0.04%     
==========================================
  Files         272      273       +1     
  Lines       15957    16132     +175     
==========================================
+ Hits        15350    15525     +175     
  Misses        607      607              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jmalles jmalles requested a review from bettina-gier December 5, 2024 09:58
@jmalles jmalles marked this pull request as ready for review December 5, 2024 10:14
@bettina-gier

Copy link
Copy Markdown
Contributor

Tried with the example recipe and it looks good, output seems as expected. Fast look through the code looks good. I'll do a more detailed review when I have more time.

@schlunma

Copy link
Copy Markdown
Contributor

Hi @jmalles, thanks for contributing!

To start the review process and finalize this, we need tests (see e.g., here) and some documentation (see e.g, here). Thanks!!

Comment thread esmvalcore/config-developer.yml
jmalles added 2 commits June 18, 2026 09:01
Removed instructions for placing data files in specific subdirectories.

@bettina-gier bettina-gier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It runs, results look fine. Might be nice to add a sample config for the filename even if it's in a flat folder.

@alistairsellar Is it possible to do some sort of manual CLA override? Jan signed on the github account but most commits are under a faulty config that can't sign. Otherwise we would have to somehow rebase the commits.

@alistairsellar

alistairsellar commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Is it possible to do some sort of manual CLA override?

Hi @bettina-gier. There's no way to override the CLA list of signatories if GitHub doesn't think that a commit is linked to a GitHub account. What I would suggest is that if you know for certain that all commits in a PR have been made by people who have signed the CLA (like in this case), then you can temporarily switch off the branch protection that looks at the CLA check. (This aligns with the spirit of why we need the CLA.) And then switch the rule back on again after the PR is merged.

Happy to do the switch-off when you're ready to merge, though I expect you've got permissions to change rules?

@bettina-gier

Copy link
Copy Markdown
Contributor

I personally don't have permissions for that I believe. I'll ask for a quick final tech review before merge!

@bouweandela bouweandela left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! To wrap it up, could you please add a bit more documentation about the various facets that can be used from the recipe to control the grid and remove code that was copy/pasted from the ICON fix but does not apply to ORAS5?

Comment thread esmvalcore/cmor/_fixes/native6/oras5.py Outdated
Comment thread esmvalcore/cmor/_fixes/native6/oras5.py Outdated
Comment thread esmvalcore/cmor/_fixes/native6/oras5.py
Comment thread esmvalcore/cmor/_fixes/native6/oras5.py Outdated
Comment thread doc/develop/fixing_data.rst Outdated
Comment thread tests/integration/cmor/_fixes/native6/test_oras5.py Outdated
Comment thread esmvalcore/cmor/_fixes/native6/oras5.py
Comment thread esmvalcore/cmor/_fixes/native6/oras5.py Outdated
Comment thread tests/integration/cmor/_fixes/native6/test_oras5.py Outdated
Comment thread tests/integration/cmor/_fixes/native6/test_oras5.py Outdated

@jmalles jmalles left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully, all comments are addressed now. Otherwise, let me know!

Comment thread tests/integration/cmor/_fixes/native6/test_oras5.py Outdated
Comment thread doc/quickstart/find_data.rst Outdated
Comment thread tests/integration/cmor/_fixes/native6/test_oras5.py
Comment thread tests/integration/cmor/_fixes/native6/test_oras5.py Outdated
@bouweandela

Copy link
Copy Markdown
Member

Hopefully, all comments are addressed now. Otherwise, let me know!

Thanks, mostly good now! I found a few more things in the tests that appear to be unused and the documentation could be a bit more clear.

@jmalles jmalles left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I addressed the comments and hope this is good to go now! 🙂

Comment thread doc/quickstart/find_data.rst Outdated

@bouweandela bouweandela left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me now

@bouweandela bouweandela merged commit 7bcf658 into main Jun 22, 2026
4 of 5 checks passed
@bouweandela bouweandela deleted the oras5 branch June 22, 2026 10:19
@bouweandela bouweandela changed the title Adding fix for ORAS5 Add support for reading ORAS5 data Jun 22, 2026
@bouweandela bouweandela added the fix for dataset Related to dataset-specific fix files label Jun 22, 2026
@jmalles

jmalles commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Thank you all, I am happy that this is finally done! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix for dataset Related to dataset-specific fix files

Projects

No open projects

Development

Successfully merging this pull request may close these issues.